[r] Add enumeration support #1559

eddelbuettel · 2023-07-28T00:23:23Z

Issue and/or context:

As described in #1558 and #866, adding enumeration support is desirable once we have TileDB Embedded 2.17 available

Changes:

This PR supports reading of columns with enumerations (aka dictionaries aka factor variable) directly via Arrow. Preliminary write support is also available (but still goes through the tiledb R package for writes).

Notes for Reviewer:

~~This PR is now work-in-progress and not ready for a merge while we await TileDB 2.17.~~ The branch and PR are ready but should only be merged once prequisites are been merged. It likely needs #1519 (C++ side) and #1663 (CI support).

CI is turned off as the TileDB default build is still without support for enumerations.

shortcut-integration · 2023-07-28T00:23:26Z

This pull request has been linked to Shortcut Story #32235: [r] Support enumerations in SOMA.

eddelbuettel · 2023-07-28T00:51:38Z

A quick snapshot of what works as of today / this branch when 2.17 is available.

Code

library(tiledbsoma)
library(tiledb)

pp <- palmerpenguins::penguins
uri <- tempfile(pattern="tiledb")

fromDataFrame(pp, uri)

## read one: soma_array_reader
p <- soma_array_reader(uri)
print(tibble::as_tibble(arrow::RecordBatch$import_from_c(p[[1]], p[[2]])))

## read twp: iterated reader for sdf
p <- SOMADataFrameOpen(uri)$read()$concat()
print(tibble::as_tibble(p))

Output

# A tibble: 344 × 9
   `__tiledb_rows` species island    bill_length_mm bill_depth_mm flipper_length_mm body_mass_g sex     year
             <int> <fct>   <fct>              <dbl>         <dbl>             <int>       <int> <fct>  <int>
 1               1 Adelie  Torgersen           39.1          18.7               181        3750 male    2007
 2               2 Adelie  Torgersen           39.5          17.4               186        3800 female  2007
 3               3 Adelie  Torgersen           40.3          18                 195        3250 female  2007
 4               4 Adelie  Torgersen           NA            NA                  NA          NA NA      2007
 5               5 Adelie  Torgersen           36.7          19.3               193        3450 female  2007
 6               6 Adelie  Torgersen           39.3          20.6               190        3650 male    2007
 7               7 Adelie  Torgersen           38.9          17.8               181        3625 female  2007
 8               8 Adelie  Torgersen           39.2          19.6               195        4675 male    2007
 9               9 Adelie  Torgersen           34.1          18.1               193        3475 NA      2007
10              10 Adelie  Torgersen           42            20.2               190        4250 NA      2007
# ℹ 334 more rows
# A tibble: 344 × 9
   `__tiledb_rows` species island    bill_length_mm bill_depth_mm flipper_length_mm body_mass_g sex     year
             <int> <fct>   <fct>              <dbl>         <dbl>             <int>       <int> <fct>  <int>
 1               1 Adelie  Torgersen           39.1          18.7               181        3750 male    2007
 2               2 Adelie  Torgersen           39.5          17.4               186        3800 female  2007
 3               3 Adelie  Torgersen           40.3          18                 195        3250 female  2007
 4               4 Adelie  Torgersen           NA            NA                  NA          NA NA      2007
 5               5 Adelie  Torgersen           36.7          19.3               193        3450 female  2007
 6               6 Adelie  Torgersen           39.3          20.6               190        3650 male    2007
 7               7 Adelie  Torgersen           38.9          17.8               181        3625 female  2007
 8               8 Adelie  Torgersen           39.2          19.6               195        4675 male    2007
 9               9 Adelie  Torgersen           34.1          18.1               193        3475 NA      2007
10              10 Adelie  Torgersen           42            20.2               190        4250 NA      2007
# ℹ 334 more rows

johnkerl · 2023-09-06T14:48:57Z

@eddelbuettel can you please resolve conflicts?

eddelbuettel · 2023-09-06T15:57:37Z

Rebased again (which was a little more involved than usual), passes 1930 tests here.

(I used to have all CI turned off waiting for the merge of TileDB Core, that regressed so some read X now.)

codecov-commenter · 2023-09-08T21:55:26Z

Codecov Report

Patch has no changes to coverable lines.

❗ Current head 513b478 differs from pull request most recent head 10a94cd. Consider uploading reports for the commit 10a94cd to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

📢 Thoughts on this report? Let us know!.

johnkerl

🚢

Pending CI of course! 😎

eddelbuettel · 2023-09-14T14:15:41Z

Pending CI of course! 😎

I should probaby update the libtiledbsoma side from the rc to the release too. Or will you take of that 'soon' (pending the unit test update) ?

(And I do expect this to go ❌ on python interop as the last ones did....) And so it happens. So I thing this needs some work somewhere else first, then a rebase and then we can talk

johnkerl · 2023-09-14T15:48:54Z

@eddelbuettel I tried #1677 as a candidate to go underneath #1519 #1511 #1559. There's a CI issue I'm puzzling over. Let's chat in Slack.

eddelbuettel · 2023-09-14T15:52:32Z

@johnkerl I am lost. Might be a candidate for outreach to the author of these contributed tests too.

johnkerl · 2023-09-14T16:38:31Z

The failing CI step is R/Python interop which is here
https://github.com/single-cell-data/TileDB-SOMA/blob/main/.github/workflows/r-python-interop-testing.yml
and it does quite simply python -m pip -v install -e apis/python, and apis/python/setup.py has tiledb (tiledb-py) dependency still 0.22.2 (you can see core 2.16.3 in the show_package_versions() output in the failed CI job). This is where we are at until TileDB-Py 0.23.0 is released. So, let's wait for that.

* [c++] Support `Enumeration` in C++ Codebase * Addition of `SOMAArray::get_enum` and `SOMAArray::get_enum_label_on_attr` * Attach an enumeration/dictionary to the `ColumnBuffer` if applicable; this is used when converting from `ArrayBuffers` to Arrow Tables in the Python and R APIs * Add `get_attr_to_enum_mapping` Function * WIP fix bug where attr name was passed instead of enum name * Add Unit Tests for Enumeration in C++ * `to_varlen_buffers` Returns `std::string` * Prior to TileDB-Inc/TileDB#4272, the SOMA unit tests were erroneously writing a byte vector for string dimensions which maps to `TILEDB_BLOB` rather than `TILEDB_STRING_ASCII` * Update SOMA Array get_metadata Signatures * Depend on 2.17.0-rc0 * resolve an incomplete merge with #1559 * [r] Update tiledb-r to RC (borrowed from #1663, #1665) * [r] Undo brown-bag typo in helper script * 2.17.0 --------- Co-authored-by: John Kerl <[email protected]> Co-authored-by: Dirk Eddelbuettel <[email protected]>

eddelbuettel · 2023-09-14T21:33:59Z

Ok, did a first rebase (which was much smother than a local one I had recently) so fingers crossed!

As described in #1558 and #866, adding enumeration support is desirable once we have TileDB Embedded 2.17 available **Changes:** This PR supports reading of columns with enumerations (aka dictionaries aka factor variable) directly via Arrow. Preliminary write support is also available (but still goes through the `tiledb` R package for writes). **Notes for Reviewer:** ~This PR is now work-in-progress and not ready for a merge while we await TileDB 2.17.~ The branch and PR are ready but should only be merged once prequisites are been merged. It likely needs #1519 (C++ side) and #1663 (CI support). CI is turned off as the TileDB default build is still without support for enumerations.

This PR adds support for return Arrow tables with dictionaries that can include `ordered` enumerations. **Changes:** Given #1559 which it depends upon, a very small change to just three files in `libtiledbsoma`. This should become clearer once the dependent PR is merged and can be rebased. **Notes for Reviewer:** [SC 34073](https://app.shortcut.com/tiledb-inc/story/34073/c-add-ordered-support-to-arrow-export)

This PR extends the `schema()` function to return an Arrow schema with enumerations including `ordered`. **Changes:** Given #1559 which it depends upon, a very small change to just one file. This should become clearer once the dependent PR is merged and can be rebased. **Notes for Reviewer:** [SC 34074](https://app.shortcut.com/tiledb-inc/story/34074/c-add-ordered-support-to-arrow-export)

* ihn/bugfix * unit-test update * lint

eddelbuettel · 2023-09-15T21:35:34Z

Ok if we all hold our breath for 20 minutes I am sure this run will turn all green....

johnkerl · 2023-09-15T21:36:38Z

Ok if we all hold our breath for 20 minutes I am sure this run will turn all green....

@eddelbuettel That's the plan! :)

eddelbuettel · 2023-09-15T21:37:13Z

Cnttlkrtksmhldngmbrthshrd

eddelbuettel · 2023-09-15T21:38:15Z

(That was "Can't type or talk as I am holding my breath" approximately free of vowels ... 😉 )

* ihn/bugfix * unit-test update * lint

eddelbuettel · 2023-09-15T21:46:11Z

Yay yay yay

Not so fast. One more.

Allow `write_soma.data.frame()` to write factors as categoricals. `write_soma()` will no longer cast factors to characters prior to writing Redo of #1683 since it got removed from #1559

* [r] Update `write_soma.data.frame()` to support categoricals Allow `write_soma.data.frame()` to write factors as categoricals. `write_soma()` will no longer cast factors to characters prior to writing Redo of #1683 since it got removed from #1559 * Handle enumerations in `write_soma.data.frame()` * Add tests

eddelbuettel requested review from mlin, aaronwolen, johnkerl, nguyenv, mojaveazure and pablo-gar July 28, 2023 00:23

eddelbuettel mentioned this pull request Jul 28, 2023

[tracking] Add support for enumerated types aka categoricals aka factors #866

Closed

eddelbuettel force-pushed the de/sc-32235/enumeration_support branch from a3960e8 to c1b070c Compare July 28, 2023 00:26

eddelbuettel force-pushed the de/sc-32235/enumeration_support branch 2 times, most recently from c45ee26 to 4eb3f90 Compare July 31, 2023 20:28

eddelbuettel force-pushed the de/sc-32235/enumeration_support branch 3 times, most recently from 9a4c341 to 407f83e Compare August 18, 2023 20:04

eddelbuettel force-pushed the de/sc-32235/enumeration_support branch 3 times, most recently from 8013da5 to 4668e2e Compare August 31, 2023 00:58

eddelbuettel force-pushed the de/sc-32235/enumeration_support branch from 4668e2e to 455182e Compare September 6, 2023 15:56

eddelbuettel force-pushed the de/sc-32235/enumeration_support branch from fd647bf to ed40f3b Compare September 6, 2023 18:53

eddelbuettel marked this pull request as ready for review September 8, 2023 13:56

eddelbuettel changed the title ~~[r] [WIP] Add enumeration support~~ [r] Add enumeration support Sep 8, 2023

eddelbuettel force-pushed the de/sc-32235/enumeration_support branch from ed40f3b to 7f9ee36 Compare September 8, 2023 20:52

eddelbuettel mentioned this pull request Sep 8, 2023

[r] Use TileDB Core RC in CI #1665

Merged

johnkerl added a commit that referenced this pull request Sep 10, 2023

resolve an incomplete merge with #1559

a1afbb9

johnkerl added a commit that referenced this pull request Sep 10, 2023

resolve an incomplete merge with #1559

5fba376

eddelbuettel mentioned this pull request Sep 13, 2023

[c++] [draft] Support Arrow ordered enumeration data on read #1674

Closed

eddelbuettel mentioned this pull request Sep 13, 2023

[r] [draft] Show Arrow schema with enumerations incl ordered #1675

Closed

johnkerl reviewed Sep 14, 2023

View reviewed changes

johnkerl approved these changes Sep 14, 2023

View reviewed changes

johnkerl mentioned this pull request Sep 14, 2023

[c++] Depend on core 2.17.0 #1677

Merged

eddelbuettel force-pushed the de/sc-32235/enumeration_support branch from 6ca5418 to de82254 Compare September 14, 2023 21:33

eddelbuettel added 3 commits September 15, 2023 15:19

johnkerl mentioned this pull request Sep 15, 2023

[c++] Merges for #1559 #1684

Merged

johnkerl added a commit that referenced this pull request Sep 15, 2023

[c++] Merges for #1559 (#1684)

0d7cb02

* ihn/bugfix * unit-test update * lint

[c++] Test fixes for #1559 (#1684)

10a94cd

* ihn/bugfix * unit-test update * lint

ihnorton force-pushed the de/sc-32235/enumeration_support branch from 0d7cb02 to 10a94cd Compare September 15, 2023 21:45

johnkerl merged commit 9b8fa52 into main Sep 15, 2023

johnkerl deleted the de/sc-32235/enumeration_support branch September 15, 2023 22:07

mojaveazure mentioned this pull request Sep 16, 2023

[r] Update write_soma.data.frame() to support categoricals #1686

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[r] Add enumeration support #1559

[r] Add enumeration support #1559

eddelbuettel commented Jul 28, 2023 •

edited

Loading

shortcut-integration bot commented Jul 28, 2023

eddelbuettel commented Jul 28, 2023

Code

Output

johnkerl commented Sep 6, 2023

eddelbuettel commented Sep 6, 2023 •

edited

Loading

codecov-commenter commented Sep 8, 2023 •

edited

Loading

johnkerl left a comment

eddelbuettel commented Sep 14, 2023 •

edited

Loading

johnkerl commented Sep 14, 2023

eddelbuettel commented Sep 14, 2023

johnkerl commented Sep 14, 2023 •

edited

Loading

eddelbuettel commented Sep 14, 2023

eddelbuettel commented Sep 15, 2023

johnkerl commented Sep 15, 2023

eddelbuettel commented Sep 15, 2023 •

edited

Loading

eddelbuettel commented Sep 15, 2023

eddelbuettel commented Sep 15, 2023 •

edited

Loading

[r] Add enumeration support #1559

[r] Add enumeration support #1559

Conversation

eddelbuettel commented Jul 28, 2023 • edited Loading

shortcut-integration bot commented Jul 28, 2023

eddelbuettel commented Jul 28, 2023

Code

Output

johnkerl commented Sep 6, 2023

eddelbuettel commented Sep 6, 2023 • edited Loading

codecov-commenter commented Sep 8, 2023 • edited Loading

Codecov Report

johnkerl left a comment

Choose a reason for hiding this comment

eddelbuettel commented Sep 14, 2023 • edited Loading

johnkerl commented Sep 14, 2023

eddelbuettel commented Sep 14, 2023

johnkerl commented Sep 14, 2023 • edited Loading

eddelbuettel commented Sep 14, 2023

eddelbuettel commented Sep 15, 2023

johnkerl commented Sep 15, 2023

eddelbuettel commented Sep 15, 2023 • edited Loading

eddelbuettel commented Sep 15, 2023

eddelbuettel commented Sep 15, 2023 • edited Loading

eddelbuettel commented Jul 28, 2023 •

edited

Loading

eddelbuettel commented Sep 6, 2023 •

edited

Loading

codecov-commenter commented Sep 8, 2023 •

edited

Loading

eddelbuettel commented Sep 14, 2023 •

edited

Loading

johnkerl commented Sep 14, 2023 •

edited

Loading

eddelbuettel commented Sep 15, 2023 •

edited

Loading

eddelbuettel commented Sep 15, 2023 •

edited

Loading